[ENH] V1 → V2 API Migration - evaluation measures #1603
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1603 +/- ##
==========================================
+ Coverage 52.04% 53.15% +1.11%
==========================================
Files 36 63 +27
Lines 4333 5032 +699
==========================================
+ Hits 2255 2675 +420
- Misses 2078 2357 +279 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
geetu040
left a comment
geetu040
left a comment
There was a problem hiding this comment.
This looks great. The implementation is clean and nicely decoupled. The next step should be adding tests, but let's wait until we have a clearer roadmap for that.
Also, please update the PR description to follow this format:
Fixes #1593
Depends on #1576
Related to #1575
EmanAbdelhaleem
force-pushed
the
evaluation-measures-mig
branch
from
f0c57de to
e091d57
Compare
This reverts commit fd43c48.
EmanAbdelhaleem
force-pushed
the
evaluation-measures-mig
branch
from
6236112 to
a19cb35
Compare
|
@geetu040 Ready for review |
- removing this since it was not part of the sdk previously - some tests fail because of the timeout in stacked PRs - this option can easily be added if needed in future
geetu040
left a comment
geetu040
left a comment
There was a problem hiding this comment.
this looks good, some code is duplicated, let's wait for the design discussion
| def test_list(self): | ||
| measures = self.resource.list() | ||
| assert isinstance(measures, list) is True | ||
| assert all(isinstance(s, str) for s in measures) is True |
There was a problem hiding this comment.
may be check if the length of list is as expected, do you think there could be a problem with that?
There was a problem hiding this comment.
Yes, if we add new evaluation measures, the length will change.
There was a problem hiding this comment.
yeah I see that, we can expect that in the test-server, so better to not hard-code specific criteria
5 participants
Fixes #1593
Depends on #1576
Related to #1575
Details
This PR implements
EvaluationMeasuresresource, and refactor its existing functions